NSX: configure segment discovery and security profiles - #13791
Conversation
dc7d4bc to
d94fafd
Compare
| InternalLbProvider, PublicLbProvider, servicepackageuuid, servicepackagedescription, PromiscuousMode, MacAddressChanges, ForgedTransmits, MacLearning, | ||
| NsxIpDiscoveryProfileId, NsxMacDiscoveryProfileId, NsxSegmentSecurityProfileId, | ||
| RelatedNetworkOffering, domainid, zoneid, pvlanType, internetProtocol |
There was a problem hiding this comment.
maybe a good moment to just format one/line? (no req, just suggestion for readability)
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #13791 +/- ##
=============================================
- Coverage 19.65% 3.41% -16.24%
=============================================
Files 6368 487 -5881
Lines 574881 41881 -533000
Branches 70351 7918 -62433
=============================================
- Hits 112970 1429 -111541
+ Misses 449639 40252 -409387
+ Partials 12272 200 -12072
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR adds support for configuring NSX segment discovery/security profiles via network offering details, validates these settings server-side, passes the selected profile IDs through the agent command path, and updates UI/i18n and unit tests accordingly.
Changes:
- Expose NSX profile ID fields (IP discovery, MAC discovery, segment security) in Add/Clone Network Offering UI and persist them via offering details.
- Validate NSX profile detail constraints (NSX-only, non-blank, no surrounding whitespace, max length) and prevent offering upgrades when profile bindings differ.
- Extend NSX segment creation to resolve/validate profile paths and bind profiles in the same hierarchical Infra request; add targeted unit tests.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/views/offering/CloneNetworkOffering.vue | Adds NSX profile input fields and submits them as offering details during clone. |
| ui/src/views/offering/AddNetworkOffering.vue | Adds NSX profile input fields and includes them in the details key set for submission. |
| ui/public/locales/en.json | Adds labels/tooltips for the new NSX profile fields. |
| server/src/test/java/com/cloud/network/NetworkServiceImplTest.java | Adds tests for offering-upgrade behavior based on NSX profile bindings. |
| server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java | Adds tests for NSX profile detail validation rules. |
| server/src/main/java/com/cloud/network/NetworkServiceImpl.java | Rejects offering upgrades when NSX segment profile bindings differ. |
| server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java | Validates NSX segment profile details (NSX-only + formatting/length rules). |
| plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxGuestNetworkGuruTest.java | Verifies profile IDs from offering details are passed into CreateNsxSegmentCommand. |
| plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java | Adds tests for binding behavior, validation failures, and “no profiles” behavior. |
| plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/resource/NsxResourceTest.java | Verifies CreateNsxSegmentCommand profile IDs are forwarded to NsxApiClient. |
| plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java | Adds overload to build CreateNsxSegmentCommand with optional profile IDs. |
| plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java | Reads offering details and passes profile IDs into segment creation command. |
| plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java | Resolves/validates profile paths and issues Infra.patch to create segment + bindings atomically. |
| plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java | Calls NsxApiClient.createSegment with the profile ID arguments. |
| plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxSegmentCommand.java | Adds profile ID fields and constructor overload for segment creation. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/network/NetworkOfferingBaseCmd.java | Documents the new NSX offering detail keys in the API parameter description. |
| api/src/main/java/com/cloud/offering/NetworkOffering.java | Adds new Detail enum values for the NSX profile ID keys. |
| PendingReleaseNotes | Adds release note entry describing NSX offering profile binding behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (values.nsxsegmentsecurityprofileid) { | ||
| params['details[' + detailsIndex + '].key'] = 'nsxsegmentsecurityprofileid' | ||
| params['details[' + detailsIndex + '].value'] = values.nsxsegmentsecurityprofileid | ||
| } |
| <a-row :gutter="12" v-if="form.provider === 'NSX'"> | ||
| <a-col :md="8" :lg="8"> | ||
| <a-form-item name="nsxipdiscoveryprofileid" ref="nsxipdiscoveryprofileid"> | ||
| <template #label> | ||
| <tooltip-label | ||
| :title="$t('label.nsx.ip.discovery.profile.id')" | ||
| :tooltip="$t('message.network.offering.nsx.ip.discovery.profile.id')"/> | ||
| </template> | ||
| <a-input v-model:value="form.nsxipdiscoveryprofileid" :maxlength="255" /> | ||
| </a-form-item> | ||
| </a-col> | ||
| <a-col :md="8" :lg="8"> | ||
| <a-form-item name="nsxmacdiscoveryprofileid" ref="nsxmacdiscoveryprofileid"> | ||
| <template #label> | ||
| <tooltip-label | ||
| :title="$t('label.nsx.mac.discovery.profile.id')" | ||
| :tooltip="$t('message.network.offering.nsx.mac.discovery.profile.id')"/> | ||
| </template> | ||
| <a-input v-model:value="form.nsxmacdiscoveryprofileid" :maxlength="255" /> | ||
| </a-form-item> | ||
| </a-col> | ||
| <a-col :md="8" :lg="8"> | ||
| <a-form-item name="nsxsegmentsecurityprofileid" ref="nsxsegmentsecurityprofileid"> | ||
| <template #label> | ||
| <tooltip-label | ||
| :title="$t('label.nsx.segment.security.profile.id')" | ||
| :tooltip="$t('message.network.offering.nsx.segment.security.profile.id')"/> | ||
| </template> | ||
| <a-input v-model:value="form.nsxsegmentsecurityprofileid" :maxlength="255" /> | ||
| </a-form-item> | ||
| </a-col> | ||
| </a-row> |
| for (NetworkOffering.Detail detail : List.of(NetworkOffering.Detail.NsxIpDiscoveryProfileId, | ||
| NetworkOffering.Detail.NsxMacDiscoveryProfileId, NetworkOffering.Detail.NsxSegmentSecurityProfileId)) { |
| protected String getIpDiscoveryProfilePath(String profileId) { | ||
| if (StringUtils.isBlank(profileId)) { | ||
| return null; | ||
| } | ||
| IpDiscoveryProfiles profiles = (IpDiscoveryProfiles) nsxService.apply(IpDiscoveryProfiles.class); | ||
| IPDiscoveryProfile profile = profiles.get(profileId); | ||
| return validateProfile(profileId, profile.getId(), profile.getPath(), "/infra/ip-discovery-profiles/", profile.getMarkedForDelete()); | ||
| } |
| import com.vmware.nsx_policy.infra.tier_1s.ipsec_vpn_services.Sessions; | ||
| import com.vmware.nsx_policy.infra.segments.SegmentDiscoveryProfileBindingMaps; | ||
| import com.vmware.nsx_policy.infra.segments.SegmentSecurityProfileBindingMaps; |
| import com.vmware.nsx_policy.model.StaticRoutesListResult; | ||
| import com.vmware.nsx_policy.model.Tag; | ||
| import com.vmware.nsx_policy.model.Tier1; | ||
| import com.vmware.nsx_policy.model.TunnelInterfaceIPSubnet; |
There was a problem hiding this comment.
I have stuff missing hence why this is a draft I will be going through everything shortly.
There was a problem hiding this comment.
no pressure (or at least you shouldn’t percieve any ;)
Summary
When no profiles are configured, segment creation keeps the existing behavior. This change is limited to NSX segment profile configuration; DHCP/IPAM and Kubernetes load-balancer rule regeneration are not part of this PR.
Validation
The branch is based on CloudStack main at
4f117071c9.Focused coverage includes:
NotFoundprofile lookup path, asserting a cleanCloudRuntimeExceptionand no segment or Infra mutation;